Middleware to handle vendor forwarded proto #193
Conversation
|
@mpalmer i think this is a great suggestion to further broaden the functionality. I’ll work on that this evening. And the resulting name change.
|
|
@mpalmer I've reworked this so it can be used to handle the transformation of any header name. And, so it can be used multiple times. Is this in line with your suggestion? I've also tried to rename it given all those changes. The tests still include X-Forwarded-Proto specific cases, but also the more generic ones. |
|
@ioquatix would love that review when you've got time. |
| # Middleware to change the name of a header | ||
| # | ||
| # So, if a server upstream of Rack sends {'X-Header-Name': "value"} | ||
| # you can change header to {'Whatever-You-Want': "value"} |
There was a problem hiding this comment.
It's probably worth describing how existing values in the "target" header will be impacted, since HTTP headers are multi-valued.
There was a problem hiding this comment.
Unless I misunderstand RFC 2616 the multiple values would come into this stage of middleware as {'Whatever-You-Want': "value1, value2"}.
I've also seen https://github.com/rack/rack/blob/ed25abcde2a64a937efa1e59f1a0bb53d7ccecb8/test/spec_files.rb#L213C10-L213C20 where the multiple values are being passed into HTTP_RANGE.
Is this the kind of multi-value you're talking about?
There was a problem hiding this comment.
If so, the change in the "Foo" header to contain "foo, bar" as the value should cover things.
There was a problem hiding this comment.
Sorry, I wasn't clear that I was referring to the situation where (for whatever reason) there was already a value in (per the example) Whatever-You-Want. The current code overwrites the existing value, and that's a reasonable behaviour, but it's worth clearly stating that in the docs, and having the tests ensure no regressions in that behaviour.
Co-authored-by: Matt Palmer <[email protected]>
Co-authored-by: Matt Palmer <[email protected]>
@mpalmer sorry to raise this separately and lose your already valuable feedback, but I'd like to own the code that I've contributed.
This is to solve the issue raised in rack/rack#2080
When routing from
cloudfront -> ALB -> puma (on ECS)we want to use the existing cloudfront header to inform rack of the protocol in use.But, we also want to keep this implementation vendor agnostic.
More context can be seen in the feedback leading to this code in rack/rack#2089
Is it best to squash my commits before any merge?
And, I assume I should add to the README?